Skip to content

Conversation

@icecrasher321
Copy link
Collaborator

@icecrasher321 icecrasher321 commented Dec 9, 2025

Summary

Improve system to use token bucket algo that prevents exploitation of boundary bursts and smoothens traffic better.

Type of Change

  • Other: Improvement

Testing

Testing Manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Dec 9, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
docs Skipped Skipped Dec 9, 2025 10:50pm

@icecrasher321 icecrasher321 marked this pull request as ready for review December 9, 2025 20:14
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 9, 2025

Greptile Overview

Greptile Summary

Replaces the fixed-window rate limiter with a token bucket algorithm to prevent exploitation of boundary bursts and smooth traffic better.

Key Changes:

  • Introduced RateLimitStorageAdapter interface with Redis and PostgreSQL implementations for token bucket state management
  • Each bucket tracks tokens (current available), lastRefillAt, and refills at refillRate per refillIntervalMs
  • Burst capacity is 2x the sustained rate (e.g., 10 req/min → 20 max burst tokens)
  • Database migration drops old user_rate_limits table and creates new rate_limit_bucket table
  • Fails closed on storage errors (denies requests rather than allowing them)
  • Updated API documentation to explain token bucket behavior with maxBurst field

Previous feedback addressed:

  • Fixed logic issue in db-token-bucket.ts (simplified to tokens >= 0)
  • Changed deprecated HMSET to HSET in Redis Lua script

Confidence Score: 4/5

  • This PR is safe to merge after verifying the migration path for existing rate limit data.
  • Well-designed token bucket implementation with clean abstractions, comprehensive tests, and proper documentation. Previous review feedback has been addressed. Score of 4 (not 5) due to the destructive migration that resets all existing rate limit state.
  • Pay attention to packages/db/migrations/0119_far_lethal_legion.sql - the migration drops the old rate limit table, which will reset all users' rate limit counters upon deployment.

Important Files Changed

File Analysis

Filename Score Overview
apps/sim/lib/core/rate-limiter/rate-limiter.ts 5/5 Complete refactor from fixed-window to token bucket algorithm. Clean abstraction using storage adapters with proper error handling (fail closed on storage errors).
apps/sim/lib/core/rate-limiter/types.ts 5/5 Config updated to use TokenBucketConfig with maxTokens (2x multiplier for burst), refillRate, and refillIntervalMs parameters.
apps/sim/lib/core/rate-limiter/storage/db-token-bucket.ts 4/5 PostgreSQL token bucket implementation using atomic INSERT...ON CONFLICT DO UPDATE. Complex SQL for token refill calculations executed in single query.
apps/sim/lib/core/rate-limiter/storage/redis-token-bucket.ts 5/5 Redis token bucket using Lua scripts for atomic token consume/status operations. Uses HSET (fixed from deprecated HMSET).
apps/sim/lib/core/rate-limiter/rate-limiter.test.ts 5/5 Comprehensive tests for token bucket rate limiter covering all trigger types, subscription plans, storage errors, and bucket reset functionality.
packages/db/migrations/0119_far_lethal_legion.sql 4/5 Creates new rate_limit_bucket table and drops old user_rate_limits table. Destructive migration - existing rate limit state will be reset.

Sequence Diagram

sequenceDiagram
    participant Client
    participant RateLimiter
    participant StorageFactory
    participant Redis/DB
    
    Client->>RateLimiter: checkRateLimitWithSubscription()
    RateLimiter->>RateLimiter: getBucketConfig(plan, counterType)
    RateLimiter->>StorageFactory: createStorageAdapter()
    StorageFactory-->>RateLimiter: Redis or DB adapter
    
    RateLimiter->>Redis/DB: consumeTokens(key, 1, config)
    
    Note over Redis/DB: Atomic operation:<br/>1. Calculate elapsed time<br/>2. Refill tokens (capped at maxTokens)<br/>3. Consume if tokens >= requested<br/>4. Return new state
    
    Redis/DB-->>RateLimiter: ConsumeResult {allowed, tokensRemaining, resetAt}
    
    alt Allowed
        RateLimiter-->>Client: {allowed: true, remaining, resetAt}
    else Rate Limited
        RateLimiter-->>Client: {allowed: false, remaining: 0, retryAfterMs}
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

19 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@icecrasher321
Copy link
Collaborator Author

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

19 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@icecrasher321 icecrasher321 merged commit aea32d4 into staging Dec 9, 2025
9 checks passed
@waleedlatif1 waleedlatif1 mentioned this pull request Dec 9, 2025
10 tasks
@waleedlatif1 waleedlatif1 deleted the feat/token-bucket-algo branch December 10, 2025 04:03
waleedlatif1 added a commit that referenced this pull request Dec 10, 2025
… docs, mcp, autolayout improvements (#2286)

* fix(mcp): prevent redundant MCP server discovery calls at runtime, use cached tool schema instead (#2273)

* fix(mcp): prevent redundant MCP server discovery calls at runtime, use cached tool schema instead

* added backfill, added loading state for tools in settings > mcp

* fix tool inp

* feat(rate-limiter): token bucket algorithm  (#2270)

* fix(ratelimit): make deployed chat rate limited

* improvement(rate-limiter): use token bucket algo

* update docs

* fix

* fix type

* fix db rate limiter

* address greptile comments

* feat(i18n): update translations (#2275)

Co-authored-by: icecrasher321 <icecrasher321@users.noreply.github.com>

* fix(tools): updated kalshi and polymarket tools to accurately reflect outputs (#2274)

* feat(i18n): update translations (#2276)

Co-authored-by: waleedlatif1 <waleedlatif1@users.noreply.github.com>

* fix(autolayout): align by handle (#2277)

* fix(autolayout): align by handle

* use shared constants everywhere

* cleanup

* fix(copilot): fix custom tools (#2278)

* Fix title custom tool

* Checkpoitn (broken)

* Fix custom tool flash

* Edit workflow returns null fix

* Works

* Fix lint

* fix(ime): prevent form submission during IME composition steps (#2279)

* fix(ui): prevent form submission during IME composition steps

* chore(gitignore): add IntelliJ IDE files to .gitignore

---------

Co-authored-by: Vikhyath Mondreti <vikhyathvikku@gmail.com>
Co-authored-by: Waleed <walif6@gmail.com>
Co-authored-by: waleedlatif1 <waleedlatif1@users.noreply.github.com>

* feat(ui): logs, kb, emcn (#2207)

* feat(kb): emcn alignment; sidebar: popover primary; settings-modal: expand

* feat: EMCN breadcrumb; improvement(KB): UI

* fix: hydration error

* improvement(KB): UI

* feat: emcn modal sizing, KB tags; refactor: deleted old sidebar

* feat(logs): UI

* fix: add documents modal name

* feat: logs, emcn, cursorrules; refactor: logs

* feat: dashboard

* feat: notifications; improvement: logs details

* fixed random rectangle on canvas

* fixed the name of the file to align

* fix build

---------

Co-authored-by: waleed <walif6@gmail.com>

* fix(creds): glitch allowing multiple credentials in an integration (#2282)

* improvement: custom tools modal, logs-details (#2283)

* fix(docs): fix copy page button and header hook (#2284)

* improvement(chat): add the ability to download files from the deployed chat (#2280)

* added teams download and chat download file

* Removed comments

* removed comments

* component structure and download all

* removed comments

* cleanup code

* fix empty files case

* small fix

* fix(container): resize heuristic improvement (#2285)

* estimate block height for resize based on subblocks

* fix hydration error

* make more conservative

---------

Co-authored-by: Vikhyath Mondreti <vikhyathvikku@gmail.com>
Co-authored-by: icecrasher321 <icecrasher321@users.noreply.github.com>
Co-authored-by: waleedlatif1 <waleedlatif1@users.noreply.github.com>
Co-authored-by: Siddharth Ganesan <33737564+Sg312@users.noreply.github.com>
Co-authored-by: mosa <mosaxiv@gmail.com>
Co-authored-by: Emir Karabeg <78010029+emir-karabeg@users.noreply.github.com>
Co-authored-by: Adam Gough <77861281+aadamgough@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants